fix(analytics): allow capability to offload reportExposure to async thread (SDK-80)#181
fix(analytics): allow capability to offload reportExposure to async thread (SDK-80)#181tylerjroach wants to merge 5 commits into
Conversation
…hread Exposure tracking on the sync path called the user's tracker (and therefore an HTTP POST) inline, blocking every getVariant / getVariantValue / isEnabled call by the full /track round trip. The async paths already use asyncio.create_task; only the sync path was paying the cost. Add an optional `exposure_executor: concurrent.futures.Executor` field to FlagsConfig. When set, the sync providers dispatch the tracker call via executor.submit so flag evaluation returns as soon as the local logic finishes. None (the default) preserves the existing inline behavior. Mirrors mixpanel-java#85. Co-Authored-By: Claude Opus 4.7 <[email protected]>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #181 +/- ##
==========================================
+ Coverage 94.50% 95.99% +1.49%
==========================================
Files 12 13 +1
Lines 1947 3299 +1352
Branches 116 224 +108
==========================================
+ Hits 1840 3167 +1327
- Misses 70 88 +18
- Partials 37 44 +7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…rage - Add an "Async Exposure Tracking" section to the openfeature-provider README showing how to configure a ThreadPoolExecutor. - Add tests covering the manual track_exposure_event API path (previously only the implicit-via-get_variant path was covered). - Add explicit tests asserting the default (None) still runs inline on the calling thread. Co-Authored-By: Claude Opus 4.7 <[email protected]>
Confidence Score: 5/5Safe to merge; the change is purely additive and the default None path is unchanged. All existing behavior is preserved by default. The only noted gap is that _log_tracker_future_exception calls future.exception() without a future.cancelled() guard, which can raise CancelledError in the done-callback when an executor is shut down with cancel_futures=True. This is a narrow edge case with no data-loss consequence. mixpanel/flags/utils.py — the _log_tracker_future_exception cancellation guard.
|
| Filename | Overview |
|---|---|
| mixpanel/flags/utils.py | Adds dispatch_exposure helper with inline/executor dispatch and done-callback for error logging; missing cancellation guard in _log_tracker_future_exception. |
| mixpanel/flags/types.py | Adds exposure_executor: Optional[Executor] = None to FlagsConfig; arbitrary_types_allowed=True already present so Pydantic handles it correctly. |
| mixpanel/flags/local_feature_flags.py | Replaces inline tracker call with _dispatch_exposure delegation; clean, no logic changes to flag evaluation. |
| mixpanel/flags/remote_feature_flags.py | Same pattern as local provider — tracker calls replaced with _dispatch_exposure; EXPOSURE_EVENT import retained (used elsewhere in the file). |
| mixpanel/flags/test_utils.py | Adds unit tests for inline dispatch and executor error-logging; good coverage of the new helper. |
| mixpanel/flags/test_local_feature_flags.py | Adds three tests covering default inline behavior and executor off-thread dispatch for local provider. |
| mixpanel/flags/test_remote_feature_flags.py | Mirrors local provider tests for remote provider; symmetric and complete. |
| openfeature-provider/README.md | Adds documentation for the new async exposure tracking option with a usage example; accurate and clear. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["get_variant / is_enabled (sync)"] --> B["_dispatch_exposure()"]
B --> C["dispatch_exposure(tracker, executor, ...)"]
C --> D{executor is None?}
D -- Yes --> E["tracker(...) inline\n(blocking, existing behavior)"]
D -- No --> F["executor.submit(tracker, ...)"]
F --> G["future.add_done_callback\n(_log_tracker_future_exception)"]
G --> H{future exception?}
H -- Yes --> I["logger.error(...)"]
H -- No --> J["done"]
F --> K["return immediately\n(non-blocking)"]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A["get_variant / is_enabled (sync)"] --> B["_dispatch_exposure()"]
B --> C["dispatch_exposure(tracker, executor, ...)"]
C --> D{executor is None?}
D -- Yes --> E["tracker(...) inline\n(blocking, existing behavior)"]
D -- No --> F["executor.submit(tracker, ...)"]
F --> G["future.add_done_callback\n(_log_tracker_future_exception)"]
G --> H{future exception?}
H -- Yes --> I["logger.error(...)"]
H -- No --> J["done"]
F --> K["return immediately\n(non-blocking)"]
Reviews (3): Last reviewed commit: "chore: fix ruff format + move Executor/F..." | Re-trigger Greptile
…ures executor.submit(...) returned a Future that was immediately discarded, so tracker exceptions raised on the executor thread disappeared with the Future. Added a done_callback that logs future.exception() at ERROR so failures are visible. Also collapsed the two identical _dispatch_exposure methods on LocalFeatureFlagsProvider and RemoteFeatureFlagsProvider into a shared utils.dispatch_exposure helper. Any future change to the dispatch policy (retry, timeout, tracing) now lives in one place. New test test_dispatch_exposure_logs_executor_thread_exceptions locks in the callback behavior — it would fail without add_done_callback.
|
Pushed P2 — silently swallowed tracker exceptions ( P2 — All 89 flag tests pass locally. |
Applies ruff format to test_utils.py and utils.py, and moves the annotation-only Executor/Future imports behind TYPE_CHECKING to satisfy TC003.
Summary
Exposure tracking on the sync path called the user's tracker — and therefore an HTTP POST — inline, blocking every
get_variant/get_variant_value/is_enabledcall by the full/trackround trip. The async paths already useasyncio.create_task; only the sync paths were paying the cost.Add an optional
exposure_executor: concurrent.futures.Executor | None = Nonefield toFlagsConfig(inherited by bothLocalFlagsConfigandRemoteFlagsConfig). When set, the sync providers dispatch the tracker call viaexecutor.submit; flag evaluation returns as soon as the local logic finishes.None(the default) preserves the existing inline behavior — no breaking change for current users.Usage
Context
Linear: SDK-80. Mirrors mixpanel-java#85. Audit-driven; same fix being applied to mixpanel-ruby and mixpanel-go in parallel PRs.
Test plan
exposure_executor=Nonekeeps the old behavior unchanged)thread_name_prefix="exposure") and not the calling thread